Repository navigation
Merge type extensions into the types they extend - #855
Open
oryan-block wants to merge 14 commits into
Open
oryan-block wants to merge 14 commits into
oryan-block wants to merge 14 commits into
Conversation
Union, enum, interface and scalar extensions subclass their base definitions, so they replaced the base type (whichever came last won) instead of being merged into it. Directives and implemented interfaces on object and input extensions were dropped. The scanner now keeps the base definitions and groups every extension by type name. It reads union members, interface fields and implemented interfaces from the extensions too. The parser merges members, values, fields, interfaces and directives like graphql-java's SchemaGenerator, and sets extensionDefinitions on every type. graphql-java rejects an extension that repeats a value, field or non-repeatable directive of its type. Here enum values, interface fields and directives keep the first one, so schemas that repeated them to work around this keep building. Object and input fields are handled as before. An extension of a different kind than its type is now an error. A union, enum, interface or scalar extension without a definition still builds as the type itself, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The scanner no longer passes them to the parser among the definitions. The enums used by directives are now also checked against the fields of interface extensions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Also checks that enums used only by directives get the values of their extensions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An undefined type that only has an object or input extension is now reported as not defined. The directives test also covers a non-repeatable directive that only an extension applies, and checks that each directive is wired once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…or enum Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt
…tensions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #552
Checklist
Description
Extending a union, enum, interface or scalar replaces the type instead of adding to it, and no error is raised. On master,
union FooBar = Foo | Barplusextend union FooBar = Bazgives members[Baz],enum Color { RED }plusextend enum Color { GREEN }gives values[GREEN], andinterface Node { id: ID }plusextend interface Node { name: String }gives fields[name]. With the usual base-then-extension order it's the base that gets lost, which is what Sceri hit: interface fields go missing and things break at runtime. Object and input extensions merge their fields, but their directives are dropped. So a federation entity declared withtype Product @key(fields: "upc")plusextend type Product @key(fields: "name")keeps only the first@key, which is the gap left by #345.extend type X implements Yis dropped too.UnionTypeExtensionDefinition,EnumTypeExtensionDefinition,InterfaceTypeExtensionDefinitionandScalarTypeExtensionDefinitionare subclasses of their base definitions.SchemaClassScanneronly filtered out object and input extensions, so the others stayed indefinitionsByNameand replaced the base definition (associateBy, last one wins).SchemaParser#createObjectandSchemaParser#createInputObjectonly readdirectivesfrom the base definition, andcreateObjectonly read itsimplements.The scanner now keeps only base definitions in
definitionsByNameand groups every extension by type name in a newtypeExtensionsmap, which goes to the parser throughScannedSchemaObjects. When discovering types it also reads union-extension members, interface-extension fields andextend type X implements Y. Everycreate*function inSchemaParsernow merges the extensions' members, values, fields and interfaces, setsextensionDefinitions, and applies the directives of the base followed by those of the extensions (SchemaParser#directivesOf), which is what graphql-java'sSchemaGeneratordoes. Scalars get their SDL description, their basedefinitionandextensionDefinitions. Object and input field handling is unchanged. There's a smallextensionsOf<T>(name)helper inUtils.kt.graphql-java rejects an extension that repeats an enum value, an interface field, a union member or a non-repeatable directive. I didn't copy that. Those schemas built on master, since the extension replaced the type, so tools keeps the first enum value, interface field or union member and logs a WARN. #863 will make them fail in 15.0.0. A declared non-repeatable directive (or a built-in one) that an extension applies again is kept once, the first one, with a WARN. Without that it'd be applied twice in the applied directives and once in the legacy ones, and
SchemaPrinterwould print invalid SDL. Undeclared directives (withallowUndeclaredDirectives) are all kept, since they might be repeatable, like an undeclared federation@key.A union, enum, interface or scalar extension with no base type isn't valid per the spec, but it built as the type on master, so I kept that. Several such extensions of one name are now merged instead of the last one winning. Object and input extensions without a base type are still ignored. An extension whose kind doesn't match its type (e.g.
type Fooplusextend union Foo) is ignored with a WARN that names the type and both kinds, and the type is built as it's defined.Left out, and worth separate issues:
@specifiedBy. Fixing that means moving scalar creation to the parser.#842, #843, #852 and #838 are merged in. #842's
findUsagenow walkstypeExtensionstoo, so its error hints name the fields ofextend interfacelike those of the interface itself. The scanner's separateextensionDefinitionsandinputExtensionDefinitionslists are gone. Object and input extensions come fromtypeExtensionslike the others and no longer go to the parser among the definitions. #852's check for enums that only directives use now also looks at the fields of interface extensions, and those enums get the values of theirextend enumlike any other enum. #854's lookup of sealed subclasses also reads the members ofextend unionandextend type X implements Y.The new
TypeExtensionTesthas 12 tests and runs queries where it matters. One schema covers merged union members, enum values (with the extension declared first), interface fields andimplements,extend type X implements Yand scalar descriptions, plusdefinitionandextensionDefinitionson each. Two more cover a nested union added by an extension and a union used only by an interface-extension field. The scanner wants classes for the members of a union and of its extensions, in both declaration orders, and for objects that implement an interface only through an extension. The parser wants enum constants for the values of the enum and of its extensions, and implementors need the interface's own fields. For directives, one schema applies a repeatable directive to an object, interface, union, enum and input object and to their extensions. It also has a non-repeatable directive that only an extension applies and one that an extension applies again. It checks the applied directives, thatSchemaDirectiveWiringfires once for each, the legacy directives and theSchemaPrinteroutput. Undeclared extension directives are checked with and withoutallowUndeclaredDirectives. It also coversincludeUnusedTypes,getUnusedDefinitions(), repeated values, fields and members, extension-only enums and unions, ignored extensions of a different kind, the error hint for an interface-extension field, and anextend enumof an enum that only a directive uses. On master 8 tests fail and 2 error. The error hint and kind mismatch tests pass there too, since master builds the interface from its extension and also ignores the mismatched extensions in that schema.clean verifypasses with 300 tests on graphql-java 26.1, and so does-Dgraphql-java.version=25.1.Behaviour change: union, enum, interface and scalar extensions now add to their type instead of replacing it. Directives on
extend type,extend input,extend interface,extend unionandextend enumare now applied and passed toSchemaDirectiveWiring, andextend type X implements Ytakes effect. Some schemas that built before now fail. Every member of a union and its extensions needs a class, every enum value of the type and its extensions must exist in the Java enum, implementors must declare the interface's own fields, and unions and interfaces used only by fields that used to be dropped are now discovered, so their members and implementors need classes too. An undeclared directive on an extension fails with "Found applied directive ... without corresponding directive definition" unlessallowUndeclaredDirectivesis set. An extension of a different kind than its type is ignored with a WARN, where on master a union, enum, interface or scalar extension like that could replace the type. In type errors, a type that only has anextend typeorextend inputis now "not defined in the schema" instead of "can't be used as an input type" (or output type). Where an extension repeats an enum value, interface field or union member, the first one declared is kept with a WARN, where master kept the last.definitionis the type's own definition, or the first extension for a union, enum, interface or scalar that only has extensions, and that's also whatgetUnusedDefinitions()reports.getExtensionDefinitions()is filled on every kind, soSchemaPrinterwithuseAstDefinitions(true)prints the extensions. Probably worth a minor version.🤖 Generated with Claude Code